-
Notifications
You must be signed in to change notification settings - Fork 216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TEP: SBT Standard #85
Conversation
text/0083-sbt-standard.md
Outdated
|
||
In case when `verify_ownership` was bounced back to SBT, SBT should send message to owner with schema: | ||
``` | ||
verify_ownership_bounced#81b510c2 query_id:uint64 sbt_id:uint256 owner:MsgAddress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does owner need sbt_id, owner, data and content in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is like proxy of bounced message, technically i think most of that it is not so useful, but just for similarity with original.
As a use case, sbt_id can be used to confirm that bounce was from real SBT, by calculating address and comparing with sender.
text/0083-sbt-standard.md
Outdated
Sender address is not an owner's address. | ||
|
||
**Otherwise should do:** | ||
Set owner's address to null and set public key to 0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need response with excess money here back to owner?
Do we want to keep enough balance on SBT to ensure it will not be frozen and destroyed for a long time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion, I think we can add reserve of 0.05 TON and return excess back, if balance is less than 0.05 throw an error. Do you think it is good approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
text/0083-sbt-standard.md
Outdated
1. `get_public_key()` - returns `int`, that is owner's public key. | ||
2. `get_nonce()` - returns `int`, which current nonce. | ||
3. `get_nft_data()` - same as in [NFT standard](https://github.com/ton-blockchain/TIPs/issues/62). | ||
4. `get_authority_address()` - returns `slice`, that is authority's address. Authority can revoke SBT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If authority can not be changed may be it is better to force collection to be authority? Not sure though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then it will require custom collection to work with SBT, with additional revoke method, not sure if it will be good.
"Drawbacks" section is missing. |
Added initiator address to verify_onwership
Added |
text/0083-sbt-standard.md
Outdated
|
||
`sbt_nonce` - nonce, required for protection of signature replay attacks. | ||
|
||
`new_owner` - address of the new owner of the SBT item, should be the same as sender's address. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we use sender's address directly, without duplicating it in the message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your case, imagine situation when we have quite sharded chain and new owner from shard 0b00000
try to pull_ownership
from sbt deployed in 0b11111
. Message from new owner will migrate to new address some time during which attacker from the same shard may be able to send exactly the same message (signature will be valid since it doesn't include anything specific to new owner) and get sbt. While it is not critical (ownership is still bound to the public key, not the address), it is bad enough to avoid such attacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify what does "address of the new owner [...] should be the same as sender's address" mean? If we need to deal with sharding in some specific way, shouldn't we put this into spec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pull_ownership
request should cover by signature the same address from which request came. Yes, in theory, we may not include sender address into the request and instead construct signed body from request payload and sender address. However in practice it is much simpler to explicitly provide payload which was signed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, i see, this makes signature verification simpler this way. In principle, we should weigh the cost of transmitting extra 32 bytes vs gas spent constructing a cell for signature, but i doubt those extra 32 bytes would make a noticeable difference.
I looked at the spec, I have a few cosmetic suggestions: 1. Owner & WalletLet's use consistent naming for owner and wallet. The SBT is designed to be bound to the owner, but leaves ability to switch the wallet in case the owner upgrades from v3 to v4 or to v5, while keeping the same private key inside. Rename pubkey method to clarify the role of the pubkey:
Rename "pull ownership" because it sounds like some authority is taking away your ownership. We use it to switch the wallet, but keep the same owner.
2. Nonce vs SeqnoSeems like doing 3. Unify revoke & destroy?Maybe the spec could be made simpler if we unify |
From the UX perspective the SBT always belongs to the wallet, so |
|
Another note: conceptually, we need to put as little logic in the contract as possible to enforce relationship between the designated parties. In our case the parties are: "the owner" (identified by a fixed pubkey) and "the authority" (maybe rename this to "issuer"?). Do we really need a concept of a "wallet" here at all? If we don't then:
NFTItem interface is cool because you control that thing with an arbitrary address (which could be plain wallet, multisig, etc.), but SBT is not really controlled by the wallet, but explicitly controlled by a fixed pubkey. |
TL-B schema of inbound message: | ||
``` | ||
prove_ownership#04ded148 query_id:uint64 dest:MsgAddress | ||
forward_payload:^Cell with_content:Bool = InternalMsgBody; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forward_payload:^Cell with_content:Bool = InternalMsgBody; | |
data:^Cell with_content:Bool = InternalMsgBody; |
?
text/0083-sbt-standard.md
Outdated
|
||
TL-B schema of inbound message: | ||
``` | ||
pull_ownership#08496845 query_id:uint64 signature:^(bits 512) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why query_id
goes before the signature? Shouldn't it be covered by the signature and it'd be easier to verify it if sig comes first?
After additional discussions standard was split to this one and NFT Ownership, and simplified. Implementation and description was changed. |
I am pretty happy with the concept of making SBT a ≈subset of NFTs. The definition "SBT = NFT where only the minter/issuer/authority can But for the security of the issuer they need to verify that the recipient's wallet is a regular wallet v3/v4 so they don't accidentally issue SBT to a transferrable "holding contract" that would effectively "wrap" SBT into a transferrable NFT and hamper the bonding property. |
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Great work, LGTM |
This TEP enters its Final Comment Period. It ends in 10 calendar days, 11.10.2022. |
LGTM |
The Final Comment Period is complete with a disposition to merge. Thanks to the authors of TEP, @xssnick, @Naltox, @EmelyanenkoK, @oleganza, and to all who took part in the discussion. This pull request will be merged now. |
Soul bound token (SBT) is a special kind of NFT which can be transferred only between its owner's accounts. For this, it stores immutable public key of the owner, and it is needed to send transfer from new address with signature in payload to change owner's address.
There is a useful type of token which allows to give social permissions/roles or certificates to some users. For example, it can be used by marketplaces to give discounts to owners of SBT, or by universities to give attestation certificates in SBT form. Mechanics with ownership proof allows to easily prove to any contract that you are an owner of some SBT.
SBT implements NFT standard interface but
transfer
should always be rejected,pull_ownership
is used instead.